Keep chat streams alive across navigation#74
Merged
mcintyre94 merged 6 commits intomainfrom Mar 15, 2026
Merged
Conversation
mcintyre94
force-pushed
the
discuss-sprite-service-management-6e54d330
branch
2 times, most recently
from
7619094 to
72d9737
Compare
|
Bug: remove(chatId:) is never called when individual chats are deleted. ChatSessionManager.remove(chatId:modelContext:) is defined to detach a VM and evict it from the cache, but no call site invokes it. Individual chat deletion goes through chatListViewModel.deleteChat() in SpriteNavigationPanel and ChatSwitcherSheet, but neither was updated to call chatSessionManager.remove(). Result: when a user deletes a chat, ChatViewModel stays in cache with WebSocket stream active, leaking a live network connection. Fix: call chatSessionManager.remove alongside deleteChat in both locations. Also missing: unit tests for ChatSessionManager and tests for .reconnecting to .streaming status transitions as required by CLAUDE.md. |
Previously processServiceStream only upgraded status from .connecting to .streaming, so a reconnect that was actively receiving data stayed stuck on .reconnecting the whole time. Extend the same transition to also fire from .reconnecting so the UI reflects that streaming is live. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
reconnectIfNeeded now captures the service status it fetches rather than discarding it. If the service is already stopped, we pass serviceAlreadyStopped to reconnectToServiceLogs so it exits after a single log fetch instead of making a redundant status API call and looping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously ChatViewModel was recreated on every chat switch, cancelling the active stream and requiring a reconnect on return. ChatSessionManager is an @observable cache keyed by chat UUID, injected app-wide, so VMs (and their streams) survive switching chats or navigating between sprites. switchToChat now looks up the cached VM instead of creating a new one. The scenePhase handler resumes all cached VMs on foreground, not just the active one. clearAllChats detaches all VMs before wiping the chat list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Call chatSessionManager.remove() on both closeChat and deleteChat in SpriteNavigationPanel and ChatSwitcherSheet, so cached VMs are detached and evicted immediately rather than leaking with an active WebSocket stream - Remove .disabled(isConnecting) on the new chat button — switching chats no longer cancels streams so the guard is unnecessary - Add ChatSessionManagerTests covering cache hit, eviction, detachAll, and isStreaming behaviour - Add runReconnectLoop status transition test (.reconnecting → .idle) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When reconnecting to a service, runReconnectLoop was using messages.last(where: .assistant) which could find an assistant message from a *previous* exchange and write the new response into it — placing it before the most recent user message. Fix: only reuse an existing assistant message if it's already at the tail of the messages array. Since each user message gets a fresh service, if the last message is a user message a new assistant message must always be created after it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mcintyre94
force-pushed
the
discuss-sprite-service-management-6e54d330
branch
from
a7c33be to
9e403e0
Compare
When the app is killed after a user message is persisted but before the service is created on the Sprite, reconnectIfNeeded would silently return leaving a dangling user bubble with no response. Now it detects a trailing user message and restores it to the input box as a draft instead. Also handles the edge case where lastSessionComplete is true but a new user message was appended before saveSession(isComplete:false) ran. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
.reconnecting→.streamingtransition:processServiceStreamonly upgraded status from.connectingto.streaming, so a reconnect that was actively receiving data stayed stuck showing "reconnecting" the whole time. Now also transitions from.reconnecting.Skip polling loop when service already stopped:
reconnectIfNeededwas fetching service status but discarding the result. Now passes it asserviceAlreadyStoppedtoreconnectToServiceLogs, which exits after one log fetch instead of making a redundant status API call and re-entering the loop.App-wide
ChatSessionManager: PreviouslyChatViewModelwas recreated on every chat switch, cancelling the active stream and requiring a reconnect on return.ChatSessionManageris a@MainActor @Observablecache keyed by chat UUID, injected into the SwiftUI environment at the app root. VMs (and their streams) now survive switching chats or navigating between sprites — connections are only torn down on explicit actions (interrupt, clearing all chats).Test plan
.streaming(not.reconnecting) once data is flowing during a reconnect🤖 Generated with Claude Code